Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Print every file exported with PCKPacker.flush()'s verbose parameter #58520

Merged
merged 1 commit into from
Feb 25, 2022
Merged

Print every file exported with PCKPacker.flush()'s verbose parameter #58520

merged 1 commit into from
Feb 25, 2022

Conversation

Calinou
Copy link
Member

@Calinou Calinou commented Feb 25, 2022 •
edited
Loading

Previously, only one line per 100 files was printed.

This also refactors the print statement to use Godot methods and make it more informative overall.

This closes #58392.

Testing project: test_pck_export.zip

Preview

extends Node


func _ready() -> void:
	var small_packer = PCKPacker.new()
	assert(OK == small_packer.pck_start("small.pck"))
	assert(OK == small_packer.add_file("res://test.png", "icon.png"))
	assert(OK == small_packer.flush(true))

Results in:

[1/1 - 100%] PCKPacker flush: icon.png -> res://test.png

@Calinou Calinou requested a review from a team as a code owner February 25, 2022 01:34
@Calinou Calinou added bug cherrypick:3.4 cherrypick:3.x Considered for cherry-picking into a future 3.x release topic:core labels Feb 25, 2022
@Calinou Calinou added this to the 4.0 milestone Feb 25, 2022
@Calinou Calinou changed the title Print every file exported with PCKPacker.flush()s verbose parameter Print every file exported with PCKPacker.flush()'s verbose parameter Feb 25, 2022
@Calinou Calinou force-pushed the pckpacker-verbose-print-for-every-file branch from 785d462 to acc289d Compare February 25, 2022 01:42
@Calinou
Print every file exported with PCKPacker.flush()s verbose parameter
8e57e5d 
Previously, only one line per 100 files was printed.

This also refactors the print statement to use Godot methods and
make it more informative overall.
@Calinou Calinou force-pushed the pckpacker-verbose-print-for-every-file branch from acc289d to 8e57e5d Compare February 25, 2022 01:43
@akien-mga
Copy link
Member

I assume this was done on purpose to prevent flooding when flushing a PCK with a lot of files. Did you test this with e.g. 10,000 files?

@Calinou
Copy link
Member Author

Calinou commented Feb 25, 2022 •
edited
Loading

I assume this was done on purpose to prevent flooding when flushing a PCK with a lot of files. Did you test this with e.g. 10,000 files?

What's interesting is that prior to this PR, exporting a PCK with 10,000 files only printed one line and not 100:

10000/10000 (100.00)

That line was also not printed to the editor Output panel, only to the terminal (since printf() was used).

Now, it does print 10,000 lines, including in the editor Output panel:

...
[9977/10000 - 99%] PCKPacker flush: icon9976.png -> res://test9976.png
[9978/10000 - 99%] PCKPacker flush: icon9977.png -> res://test9977.png
[9979/10000 - 99%] PCKPacker flush: icon9978.png -> res://test9978.png
[9980/10000 - 99%] PCKPacker flush: icon9979.png -> res://test9979.png
[9981/10000 - 99%] PCKPacker flush: icon9980.png -> res://test9980.png
[9982/10000 - 99%] PCKPacker flush: icon9981.png -> res://test9981.png
[9983/10000 - 99%] PCKPacker flush: icon9982.png -> res://test9982.png
[9984/10000 - 99%] PCKPacker flush: icon9983.png -> res://test9983.png
[9985/10000 - 99%] PCKPacker flush: icon9984.png -> res://test9984.png
[9986/10000 - 99%] PCKPacker flush: icon9985.png -> res://test9985.png
[9987/10000 - 99%] PCKPacker flush: icon9986.png -> res://test9986.png
[9988/10000 - 99%] PCKPacker flush: icon9987.png -> res://test9987.png
[9989/10000 - 99%] PCKPacker flush: icon9988.png -> res://test9988.png
[9990/10000 - 99%] PCKPacker flush: icon9989.png -> res://test9989.png
[9991/10000 - 99%] PCKPacker flush: icon9990.png -> res://test9990.png
[9992/10000 - 99%] PCKPacker flush: icon9991.png -> res://test9991.png
[9993/10000 - 99%] PCKPacker flush: icon9992.png -> res://test9992.png
[9994/10000 - 99%] PCKPacker flush: icon9993.png -> res://test9993.png
[9995/10000 - 99%] PCKPacker flush: icon9994.png -> res://test9994.png
[9996/10000 - 99%] PCKPacker flush: icon9995.png -> res://test9995.png
[9997/10000 - 99%] PCKPacker flush: icon9996.png -> res://test9996.png
[9998/10000 - 99%] PCKPacker flush: icon9997.png -> res://test9997.png
[9999/10000 - 99%] PCKPacker flush: icon9998.png -> res://test9998.png
[10000/10000 - 100%] PCKPacker flush: icon9999.png -> res://test9999.png

Since it's a verbose mode that you enable explicitly, I think it makes sense here. For 4.0, we could also consider removing this verbose argument and relying on OS.is_stdout_verbose() instead. We could even allow toggling verbose mode at run-time if having some parts of the code be verbose only is desired.

@akien-mga akien-mga merged commit 6fbfb27 into godotengine:master Feb 25, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.5.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Mar 1, 2022
@akien-mga
Copy link
Member

Cherry-picked for 3.4.4.

@Calinou Calinou deleted the pckpacker-verbose-print-for-every-file branch March 1, 2022 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akien-mga akien-mga akien-mga approved these changes

Assignees
No one assigned
Labels
bug topic:core
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

Docs for PckPacker flush verbose param are misleading
2 participants
@Calinou @akien-mga